Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/6.0] <EnablePreviewFeatures> for HttpStress #60682

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 20, 2021

Backport of #59410 to release/6.0

/cc @antonfirsov

This is a test-only change fixing a build error in HttpStress so we can run stress tests again on release/6.0.

Example failing build on the release/6.0 branch:
https://dev.azure.com/dnceng/public/_build/results?buildId=1429667&view=logs&j=2d2b3007-3c5c-5840-9bb0-2b1ea49925f3&t=8ca65746-3bfa-57bf-9141-d805b5d39db5

Customer Impact

We need stress tests to assure the quality of the HTTP stack in .NET 6.0.

Testing

CI run should show if the build error is gone.

Risk

None.

@ghost
Copy link

ghost commented Oct 20, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #59410 to release/6.0

/cc @antonfirsov

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@antonfirsov antonfirsov requested a review from a team October 20, 2021 12:57
@antonfirsov
Copy link
Member

/azp run runtime-libraries stress-http

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lewing lewing added this to the 6.0.0 milestone Oct 21, 2021
@danmoseley
Copy link
Member

@antonfirsov no special approval required to commit this. However, there's a cut off in this branch for 6.0 GA at 4pm PST Friday. After that the branch likely needs to hold all changes for a bit.

@antonfirsov
Copy link
Member

antonfirsov commented Oct 21, 2021

However, there's a cut off in this branch for 6.0 GA at 4pm PST Friday. After that the branch likely needs to hold all changes for a bit.

@danmoseley In that case it would be really important to merge this and #60723 before that deadline. Latter PR still needs approval. Unfortunately I don't have rights to merge into this branch. Can you help me by pinging someone who can do the merge? after the approval of #60723, which will likely happen tomorrow during Prague working hours EDIT: done
-- #60723 (review).

@danmoseley
Copy link
Member

@antonfirsov would it make sense to just hold this and the other one until the branch opens again for 6.0.1, next month? If it's just enabling more testing, and won't make the last build tomorrow any better?

We'll no doubt be taking changes into this branch for tests and such things for the full 3 years that 6.0 will be in support. tomorrow is just the cutoff for 6.0.0.

@danmoseley danmoseley added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 22, 2021
@danmoseley
Copy link
Member

Let's wait until branch opens for 6.0.1

If you need to do stress testing eg of the latest msquic please do so in main or locally until then

@jeffhandley jeffhandley modified the milestones: 6.0.0, 6.0.x Nov 22, 2021
@karelz karelz removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 22, 2021
@karelz
Copy link
Member

karelz commented Nov 22, 2021

Removing NO MERGE as it is now ready for merge into 6.0 servicing.

@danmoseley does it need Approval label to be merged? Or how does it work for test-only changes?

@karelz karelz closed this Nov 22, 2021
@antonfirsov
Copy link
Member

@karelz did you close this on purpose? Is there something that prevents us merging this?

@karelz
Copy link
Member

karelz commented Nov 22, 2021

Nope, that was a mistake, sorry and thanks for catching it.

@karelz karelz reopened this Nov 22, 2021
@karelz
Copy link
Member

karelz commented Nov 30, 2021

@danmoseley ping?

@danmoseley
Copy link
Member

Oops, missed this. No approval needed for test-only changes. As long as the branch is open.

@antonfirsov
Copy link
Member

/azp run runtime-libraries stress-http

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@antonfirsov
Copy link
Member

@danmoseley what is the process for getting this and #60723 merged? Should we apply some label, or ping some people? Note that both PR-s are needed to fully fix stress runs.

@danmoseley
Copy link
Member

Just ask @safern whether branch is open and he can merge it. Or else I can.

@safern
Copy link
Member

safern commented Dec 2, 2021

The branch doesn't open until around December 17th once the branding changes are open. If this is approved we can add servicing-approved label and I'll handle the merge once the branch is open.

@safern safern added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Dec 2, 2021
@safern
Copy link
Member

safern commented Dec 2, 2021

If this is approved we can add servicing-approved label and I'll handle the merge once the branch is open.

Ah this is a test only change. I'll merge it once the branch is open.

@antonfirsov
Copy link
Member

@safern thanks! Is there a way to put a reminder on both this PR and #60723 to make sure they get merged? Maybe apply servicing-approved regardless?

@safern
Copy link
Member

safern commented Dec 2, 2021

I will look at all PRs based on target branch and milestone, so no worries, it will get merged.

@safern safern merged commit 48d8d30 into release/6.0 Dec 15, 2021
@safern safern deleted the backport/pr-59410-to-release/6.0 branch December 15, 2021 19:29
@safern safern removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Dec 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jan 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants